Do these appear somewhere in the output? I've never used the composite actions feature, so I'm not super familiar with it. I'd think these need updating, but maybe it doesn't matter?

No, these are used when publishing standalone actions (in the repo root) to Marketplace. It's not limited actions being composite.

I think the missing fields were making either jsonschema or actionlint unhappy so I added dummy strings. They aren't actually needed for the action to function.

So far, this embedded script, and the other ones like it in ci.yml, are the thing which I find most surprising in this PR.
If I'm reading correctly, the near-equivalent bash would be this?

      hash='${{
          hashFiles(
            'tox.ini',
            'pyproject.toml',
            '.pre-commit-config.yaml',
            'pytest.ini',
            'dependencies/**',
            'dependencies/*/**',
            'setup.cfg'
          )
      }}'
      echo "computed hash is $hash"
      echo "files-hash-key=$hash" >> "${GITHUB_OUTPUT}"

I have no objection to using Python over bash as the shell for a step, but I was reading it with the expectation that I'd see some Python-specific feature here. It is more verbose in this case and I'm not sure I understand why you made this choice -- am I missing something important that I should note?

If the explanation is merely a matter of preference, then let's mark this resolved, that works for me. But I didn't want to overlook some essential detail here.

You're right. There's nothing special about this. I think that originally, I was trying to unify bits of caching, even before I distilled some into https://github.com/marketplace/actions/cache-python-deps, I was trying to separate components and reduce job sizes (it seemed necessary at the time). So I came up with a part that computes the file-based portion of the cache key and the Python runtime-derived one. Eventually, it turned out that this only needs to be called once but it was already separated and I was synchronizing changes across projects so I left it as is.

The Python syntax side of this likely came from me copying some scaffolding from other steps where I was already using Python. By the way, that second cache key portion does access version data through Python imports. I don't remember but maybe they were combined at some point prior to refactoring.

Anyway, I was thinking that it'd be good to replace GHA's hashFiles() w/ pure Python (and filter out the non-existing file path wildcards dynamically), turning this into an external action later — maybe we'd just have a list of all most used file patterns and it would be called w/o inputs even.

I haven't fully worked this out yet, but I think we should look for a way to make the hook mechanism easier to follow. The "double delegation" pattern (we delegate to reusable-tox, which delegates back to this action) results in some difficult to understand logic here in order to extract values.

i.e. We need fromJSON(inputs.job-dependencies-context).pre-setup.outputs.release-requested, which is a pretty long expression, in order to get an input which we had in-hand in a workflow in this repo (when we did the pre-setup step).

I think I would prefer if it were the responsibility of the ci.yml workflow to specify data to pass to the hooks, and then thread that down into this job. I haven't explored if this is possible, but something like...

# in ci.yml
 build:
    name: >-
      📦 Build dists
    needs:
    - pre-setup
    uses: tox-dev/workflow/.github/workflows/reusable-tox.yml@...
    with:
      # is it possible to make this something we pass in? (I'm guessing not, but it's an idea)
      post-tox-action: .github/workflows/reusables/reusable-tox/post-tox/action.yml
      # this is the important part: passing a blob of action inputs
      post-tox-action-call-args:
        release-requested: ${{ jobs.pre-setup.outputs.release-requested }}
        ...

# in action.yml , using that data
      retention-days: >-
        ${{
            fromJSON(
              fromJSON(inputs.call-args).release-requested
            )
            && 90
            || 30
        }}

It's not more powerful than having full access to the needs job context data, but I think the explicit arg-passing style is much easier to follow. I'm not sure how much of this is possible within the constraints of GitHub Actions -- it's been a while since I tinkered with custom actions, but I'm aware that there are some limitations on the system which could make this difficult.

All of the above is more of a comment on reusable-tox than it is about the integration here. I've followed enough to understand that we're getting context data from the original ci.yml passed down into here and we can move forward with this. But it was difficult to follow and I think it may be hard for future contributors to read.

Yeah, we can work on this in the reusable workflow. Some things (like passing an arbitrary hook path) are indeed impossible within the native constraints of GHA. They could probably be hacked through manual repo cloning but that's a can of worms.

I wasn't sure about passing the hook args but maybe we can think of something later. Your example of a nested input might not work natively and you'd have to turn it into JSON (or a YAML string, via |, perhaps).

The two scripts above seem significantly longer than their bash equivalents. More than in the case of the hashing action, I wonder if these should be bash shell steps, and then the tomllib and setuptools_scm scripts below can set shell: python.

Yes, bash is often shorter.

This job is about computing a lot of things so I wanted to make use of Python more w/o being caught in potential Bash traps. Plus it's in sync with other repos, which would mean having to do this in other places at the same time.

It may be a good idea to look into making it more reusable too at some point.

Seeing this snippet makes me think of a tool I've been working on which might interest you:
https://mddj.readthedocs.io/en/latest/

The advantage over calling into setuptools_scm directly is that it's generic, so if you wanted a version of this script which you can port across projects, it might look like...

# /// script
# dependencies = ["mddj==0.4.2"]
# ///
from mddj.api import DJ
dj = DJ()
ver = dj.read.version()
... # rest of script follows

And that will read project.version if it's static, or call into build if it's dynamic.

The project still needs a lot of work; it's just an alpha right now. But I find I frequently want to be able to script around project metadata, so it's an interesting space to try to make a good utility.

Yeah, this looks fun. The thing is that this snippet is supposed to predict what version is to be expected from Git so that it can be used for sanity checks in other jobs that actually grab the metadata. Perhaps, it's fine to move to reading the metadata but up until recently, setuptools-scm didn't have an easy way to let us influence its settings from outside the config. In some projects, I was modifying pyproject.toml on disk, in multiple jobs and would --assume-unchanged it in Git. There's 3 version modes this (in combination with other steps) was supposed to achieve:

• release (an exact matching tag on the same commit; sometimes emulated — in case of workflow_dispatch the tag is created locally but then actually pushed to GitHub way later)
• merge to the main branch (a version that has the .dev counter but no local portion)
• PR run (both .dev counter and the local version portion)

The first two are publishable to Test(PyPI). But since I wasn't copying all the bits from other repos just yet, this may not be as obvious.

P.S. It's always scary to modify these steps because I often mess up there with the context spread across multiple lines. So I'm usually trying to avoid that until I can turn it into something reusable.

2 minutes seems a bit tight, if there's any slowdown (e.g., network flakiness). Should we maybe loosen it a little, e.g. to 5 minutes?

For this one, this shouldn't be necessary. It takes 30s and having 4x more time is already more than enough. 1-2 extra mins for network flakiness has shown good results in the past. But I'm usually playing with longer timeouts for tests and linters, though. For example, things like sphinx's linkcheck.

This reminds me, I'd like to incorporate check-sdist as well. We can do that in a follow-up.

Sounds good. I haven't used that one so would need a demo PR or smth.

Current docs indicate that -I implies -E and -s. So I think we can cut this down to just -I?

I started implementation of this in Cheroot originally, when I was trying to handle Python 2.7 too. This needs to be cleaned up across multiple projects but I never took time to compare implications so I'm copying this around.

Can we do the cleanup in a follow-up?

I have somewhat mixed feelings about this block of config. On the one hand, I can see how it documents the short flags we use. And I didn't know you could have a custom section in tox.ini loaded like this, which is neat!

But I also find that it makes the usage sites harder to read in some sense, since you have to lookup this table to figure out what the command is. (adding a bit of indirection)
The comparison is between

{envpython} \
    -I -bb -Werror

and

{envpython} \
    {[python-cli-options]byte-errors} \
    {[python-cli-options]max-isolation} \
    {[python-cli-options]warnings-to-errors}

I'm okay with it, at least to try out, but I'm just noting that I'm not fully convinced as of yet.

Yeah, I just wanted the same things reused across the config. Also, when running tox run -v, you'll see the rendered commands so this should be enough, despite indirection. Additionally, there's tox config that shows all the settings in the resolved state.

When copying these flags, I find it helpful to see visually that I haven't forgotten about them and it's a part of the idiom for invoking Python, with all the indents and EOL escaping in the config.

I think that it may be a bit confusing when seen first but later, it feels rather natural to see all the invocations across the config "protected". I don't remember a time when I needed to look up which flags actually get rendered, unless something wasn't working, at which time I'd have to go into a deep inspection mode anyway...